Skip to content

vtorc: cleanup discover queue, add concurrency flag#17825

Merged
deepthi merged 19 commits intovitessio:mainfrom
timvaillancourt:vtorc-discover-queue-cleanup
Mar 5, 2025
Merged

vtorc: cleanup discover queue, add concurrency flag#17825
deepthi merged 19 commits intovitessio:mainfrom
timvaillancourt:vtorc-discover-queue-cleanup

Conversation

@timvaillancourt
Copy link
Contributor

@timvaillancourt timvaillancourt commented Feb 19, 2025

Description

This PR cleans up the VTOrc discover queue code, mainly by removing all locking from .Consume(); this was achieved by adding type queueItem struct and pushing this to the channel instead, with the key and time.Time included. Previously this time.Time was stored in a map with mutex locks

The .QueueLen() now returns just the len() of the queue channel. Before this metric would include the length of the channel plus all that are "in processing" (between .Consume() and .Release()), which I'm unsure is necessary. Simplifying this removed the need for a lock in .QueueLen()

Finally, the --discovery-workers flag was added to control how many workers consume the discovery queue

Benchmark:

tvaillancourt@tvailla-ltmawfe vitess % go test -v -bench=. ./go/vt/vtorc/discovery/...
=== RUN   TestQueue
--- PASS: TestQueue (0.00s)
goos: darwin
goarch: arm64
pkg: vitess.io/vitess/go/vt/vtorc/discovery
cpu: Apple M3 Max
BenchmarkQueues
BenchmarkQueues/New
BenchmarkQueues/New-14         	    4726	    244573 ns/op
BenchmarkQueues/Legacy
BenchmarkQueues/Legacy-14      	    3610	    338468 ns/op
PASS
ok  	vitess.io/vitess/go/vt/vtorc/discovery	3.923s

NOTE: New == this PR, Legacy == current queue

Related Issue(s)

#17330

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: VTOrc Vitess Orchestrator integration Type: Enhancement Logical improvement (somewhere between a bug and feature) Type: Internal Cleanup Type: Performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants